Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Improved Script Efficiency and Readability #8065

Closed
wants to merge 1 commit into from

Conversation

ursulabauer
Copy link

PR description

I’ve made a few updates to the script to improve its efficiency and readability, as well as fix some minor issues. Here's a breakdown of what’s been changed:

  • Architecture Detection: I added a line to automatically detect the system architecture (export architecture=$(uname -m)), which helps if the architecture variable isn't provided earlier.

  • Modern Syntax for Incrementing: Replaced the outdated syntax (i=\expr $i + 1) with the more modern ((i++)). This not only improves readability but also enhances the script’s performance.

  • Grammar Fix: I corrected a grammatical issue in the comments by updating the line:

    # we test that things listen on the right interface/port, not what interface they advertise
  • Replacing Inefficient Constructs: Switched the use of expr to ((i++)) for numerical operations, which is more efficient in Bash.

  • Error Checking Improvements: Changed [[ $i != 0 ]] to ((i != 0)) for checking the value of i, as the latter is more appropriate and standard for numeric comparisons in Bash.

  • Directory Creation: Added a step to ensure that the necessary report directories exist (mkdir -p ./reports) or created them if they were missing. This prevents errors when generating reports.

These updates should make the script more stable, modern, and easier to work with.

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests

@macfarla
Copy link
Contributor

Hi, thanks for your interest in our project. We'd love for you to contribute code, however we don't accept trivial contributions such as this. Have a look at good first issues and please read our contributing guidelines https://github.com/hyperledger/besu/blob/main/CONTRIBUTING.md#guidelines-for-non-code-and-other-trivial-contributions

@macfarla macfarla closed this Dec 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants